Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Least Squares RoR computation #503

Merged
merged 1 commit into from
Sep 6, 2020
Merged

Conversation

PDWest
Copy link
Contributor

@PDWest PDWest commented Sep 5, 2020

Change from estimating RoR based on the window extremal points to doing a full least squares solution using all the samples in the window

Change from estimating RoR based on the window extremal points to doing a full least squares solution using all the samples in the window
@PDWest
Copy link
Contributor Author

PDWest commented Sep 5, 2020

This PR address issue #502 that I just opened

@MAKOMO
Copy link
Member

MAKOMO commented Sep 6, 2020

Dear Phil, thanks for this excellent your contribution. I had it already on the list for the but next version of Artisan to investigate the use of polyfits for better RoR computations as now Artisan allows for delta spans larger than 1. I now experimented a little bit with your approach and it seems to reduce the noise in some cases and results in somewhat smoother RoR curves without adding any additional delay. Especially in case the original signal had a lot high-frequency noise and the delta span is set large enough such that this approach can play off. I also discovered some cases where the previous approach resulted in some spots in better results, but those are few. I will migrate this to the newer poly module of numpy that should return more stable results and also apply this to the offline case. I will also add an "optimized" offline version that avoids any timeshift caused by the RoR calculation, active if the "optimized smoothing" flag is clicked based on the scipy savitzky golay filter.

@MAKOMO MAKOMO merged commit 112fd7e into artisan-roaster-scope:master Sep 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants